Skip to content

Conversation

@nascheme
Copy link
Member

@nascheme nascheme commented Apr 28, 2017

This is a recreation of pull request #1333 as there seems no way to move the source branch into my own repo. I have removed the redundant definitions as pointed out by @tiran. I could maybe be convinced that socket.socket() get this functionality as suggested.

Comment from @tiran copied here::

I'm -1 on fromfd2. We shouldn't introduce yet another function and bloat the API even further. Instead
let's fix the underlying bug and make socket.socket(fileno=fd) detect the socket values, see
https://bugs.python.org/issue28134

Copy link
Contributor

@DimitrisJim DimitrisJim Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the versionadded (and similarly for fdtype) be 3.7?

@tiran
Copy link
Member

tiran commented Apr 28, 2017

#1349 implements a fix that does not require a new API and fixes all existing usage of socket(fileno) automatically.

Get socket information from a file descriptor.  The return value is a
3-tuple with the following structure: (family, type, proto).  Not
supported on all platforms.
@nascheme nascheme changed the title bpo-27377: Add socket.fromfd2() and socket.fdtype() bpo-27377: Add socket.fdtype() Apr 28, 2017
@nascheme
Copy link
Member Author

Changed title, I think with the socket.socket change proposed by @tiran, we don't need fromf2(). I still think socket.fdtype() is potentially useful.

As suggested by @DimitrisJim, I have fixed the version number to be 3.7.

The HAVE_FDTYPE define may need some tweaking yet. Based on testing I did some months ago, platforms define SO_TYPE and SO_PROTOCOL but the fdtype() implementation doesn't work on them.

Having this function in socket would allow the unit tests for #1349 to check for it and only run the tests if fdtype() is supported on the platform.

}

l = sizeof(sa);
if (getsockname(fd, &sa, &l) < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code doesn't work on Windows for IPv6 addresses. You have to make the buffer large enough to fit all possible address types in it.

sock_addr_t addrbuf;
socklen_t addrlen = sizeof(sock_addr_t);

memset(&addrbuf, 0, addrlen);
if (getsockname(fd, SAS2SA(&addrbuf), &addrlen) == 0) {
    family = SAS2SA(&addrbuf)->sa_family;
} else {
    return set_error();
}

@brettcannon brettcannon added the type-feature A feature request or enhancement label May 1, 2017
@nascheme
Copy link
Member Author

nascheme commented Feb 7, 2019

Closing because Christian Heimes wants to fix a different way (which sounds fine to me).

@nascheme nascheme closed this Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants